Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add akegata denki device one #9690

Merged
merged 13 commits into from
Jul 28, 2020
Merged

Add akegata denki device one #9690

merged 13 commits into from
Jul 28, 2020

Conversation

kmani314
Copy link
Contributor

@kmani314 kmani314 commented Jul 9, 2020

Description

Add the 明け方電気 (akegata denki) device one.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

zvecr
zvecr requested changes Jul 9, 2020
View reviewed changes
Copy link
Member

@zvecr zvecr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, could you please confirm which STM32F042 this keyboard runs. It might be a case were we can avoid introducing board files.

keyboards/device_one/device_one.h Outdated Show resolved Hide resolved
keyboards/device_one/device_one.h Outdated Show resolved Hide resolved
keyboards/device_one/device_one.c Outdated Show resolved Hide resolved
@zvecr zvecr added the keyboard label Jul 9, 2020
@zvecr zvecr requested a review from a team July 9, 2020 23:04
@zvecr zvecr requested a review from a team July 9, 2020 23:07
fauxpark
fauxpark requested changes Jul 10, 2020
View reviewed changes
keyboards/device_one/config.h Outdated Show resolved Hide resolved
keyboards/device_one/keymaps/ansi_arrow_keys/keymap.c Outdated Show resolved Hide resolved
keyboards/device_one/keymaps/ansi_split_backspace/keymap.c Outdated Show resolved Hide resolved
keyboards/device_one/keymaps/default/keymap.c Outdated Show resolved Hide resolved
keyboards/device_one/keymaps/iso/keymap.c Outdated Show resolved Hide resolved
@kmani314
Copy link
Contributor Author

Is the matrix scanning code in device_one.c not required? I removed it, but isn't this the code that actually registers the keypresses?

@fauxpark
Copy link
Member

matrix_scan_kb() is already weakly defined in QMK core, to do the exact same thing:
https://github.com/qmk/qmk_firmware/blob/master/quantum/matrix_common.c#L23

@kmani314
Copy link
Contributor Author

@zvecr Forgot to mention, this board runs the STM32F042K6T6

noroadsleft
noroadsleft requested changes Jul 18, 2020
View reviewed changes
Copy link
Member

@noroadsleft noroadsleft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitHub tip: You can use the Files Changed tab to apply multiple suggestions to one commit.

keyboards/device_one/info.json Outdated Show resolved Hide resolved
keyboards/device_one/config.h Outdated Show resolved Hide resolved
keyboards/device_one/device_one.h Outdated Show resolved Hide resolved
keyboards/device_one/info.json Outdated Show resolved Hide resolved
keyboards/device_one/info.json Outdated Show resolved Hide resolved
keyboards/device_one/device_one.h Outdated Show resolved Hide resolved
keyboards/device_one/keymaps/ansi_arrow_keys/keymap.c Outdated Show resolved Hide resolved
keyboards/device_one/keymaps/ansi_arrow_keys/keymap.c Outdated Show resolved Hide resolved
keyboards/device_one/keymaps/ansi_split_backspace/keymap.c Outdated Show resolved Hide resolved
keyboards/device_one/keymaps/ansi_split_backspace/keymap.c Outdated Show resolved Hide resolved
@noroadsleft
Copy link
Member

Also, since you're planning a device two, you could use a vendor folder for organization (akegata_denki/device_one/, akegata_denki/device_two/, etc.).

noroadsleft
noroadsleft requested changes Jul 22, 2020
View reviewed changes
keyboards/akegata_denki/device_one/device_one.h Outdated Show resolved Hide resolved
keyboards/akegata_denki/device_one/keymaps/iso/keymap.c Outdated Show resolved Hide resolved
keyboards/akegata_denki/device_one/keymaps/iso/keymap.c Outdated Show resolved Hide resolved
keyboards/akegata_denki/device_one/rules.mk Show resolved Hide resolved
keyboards/akegata_denki/device_one/readme.md Outdated Show resolved Hide resolved
@drashna drashna requested review from drashna and a team July 24, 2020 07:15
@noroadsleft noroadsleft merged commit f5fe6fe into qmk:master Jul 28, 2020
@noroadsleft
Copy link
Member

Thanks!

nicocesar pushed a commit to nicocesar/qmk_firmware that referenced this pull request Aug 12, 2020
@kmani314 @nicocesar
[Keyboard] Add akegata denki device one (qmk#9690)
a2813d7 
* add device one

* fix layouts

* add other layouts

* column fixes, device id changes

* layouts cleanup, add readme

* add info.json, fix readme

* add template readme

* add smaller image

* fix image links, address requested changes

* Apply suggestions from code review

* Update keyboards/device_one/keymaps/ansi_split_backspace/keymap.c

* add akegata vendor folder

* Apply suggestions from code review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zvecr zvecr zvecr requested changes

@noroadsleft noroadsleft noroadsleft approved these changes

@fauxpark fauxpark fauxpark approved these changes

@drashna drashna Awaiting requested review from drashna

Assignees
No one assigned
Labels
keyboard
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kmani314 @fauxpark @noroadsleft @drashna @zvecr